Skip to content

Address high-priority code review findings#1

Open
Steventanardi wants to merge 2 commits into
mainfrom
claude/code-review-improvements-SenKU
Open

Address high-priority code review findings#1
Steventanardi wants to merge 2 commits into
mainfrom
claude/code-review-improvements-SenKU

Conversation

@Steventanardi

Copy link
Copy Markdown
Owner
  • Remove hardcoded plaintext PINs from source; store PBKDF2 hashes
    (SHA-256, 210k iterations, per-user salt) in persisted state.
    Login now runs a first-time "Create Passcode" flow for users
    without a stored hash, then verifies via constant-time compare.
  • Add missing updateTransaction action in the Zustand store; editing
    transactions from TransactionForm no longer silently fails.
  • Validate amount input in TransactionForm (finite, positive, bounded)
    with an inline error instead of silently dropping submissions.
  • Drop the dummy Supabase URL/key fallback. Export null when env vars
    are unset and guard sync calls so misconfigured deployments fail
    loudly rather than sending traffic to your-project.supabase.co.
  • Stop persisting currentUser to localStorage. Auth must be
    re-established on reload, matching the existing 1-minute idle
    timeout in App.jsx.
  • Delete src/context/AppContext.jsx. It was orphaned (no imports
    anywhere), duplicated the Zustand store, and contained an
    unguarded JSON.parse on line 61.

https://claude.ai/code/session_0148Nj3thTE1VdR5wcxzHyLz

claude added 2 commits April 21, 2026 16:27
- Remove hardcoded plaintext PINs from source; store PBKDF2 hashes
  (SHA-256, 210k iterations, per-user salt) in persisted state.
  Login now runs a first-time "Create Passcode" flow for users
  without a stored hash, then verifies via constant-time compare.
- Add missing updateTransaction action in the Zustand store; editing
  transactions from TransactionForm no longer silently fails.
- Validate amount input in TransactionForm (finite, positive, bounded)
  with an inline error instead of silently dropping submissions.
- Drop the dummy Supabase URL/key fallback. Export null when env vars
  are unset and guard sync calls so misconfigured deployments fail
  loudly rather than sending traffic to your-project.supabase.co.
- Stop persisting currentUser to localStorage. Auth must be
  re-established on reload, matching the existing 1-minute idle
  timeout in App.jsx.
- Delete src/context/AppContext.jsx. It was orphaned (no imports
  anywhere), duplicated the Zustand store, and contained an
  unguarded JSON.parse on line 61.

https://claude.ai/code/session_0148Nj3thTE1VdR5wcxzHyLz
- App.jsx idle-timeout rewrite. Extract IDLE_TIMEOUT_MS /
  IDLE_CHECK_INTERVAL_MS / ACTIVITY_THROTTLE_MS / RATE_REFRESH_MS
  constants. Only call setLastActive when the tab is visible,
  and throttle it to at most once per 5s so it stops hammering
  the Zustand store on every keypress. Attach visibilitychange
  to document (it is a document event — the old window listener
  was a no-op) and use it to re-check the timeout on tab return
  instead of silently resetting lastActive.

- Modal.jsx: save and restore the body's original overflow/height
  instead of blindly writing "unset", which clobbered any caller-
  supplied styles. Drop overflowY:'visible' on the modal content,
  which contradicted the inner scrolling container on line 82.

- Delete test_ocr.js — ad-hoc script with a hardcoded Windows path
  ('d:\\Money Planner\\test_receipt.png'). Not part of the build
  and not runnable on other machines.

- Fix eslint.config.js: eslint-plugin-react-hooks 5.x no longer
  exposes configs.flat.recommended; use configs['recommended-
  latest'] so lint actually runs. (Surfaces 28 pre-existing
  warnings in files outside this PR — left as-is.)

- Add Vitest + jsdom with "test" / "test:watch" scripts and a
  vitest config block in vite.config.js. Cover crypto helpers
  (salt uniqueness, round-trip verify, malformed stored entry
  handling) and the store's transaction + PIN actions (12 tests,
  all passing locally).

- CI: run `npm test` between lint and build so regressions in
  store actions or crypto helpers block merges.

https://claude.ai/code/session_0148Nj3thTE1VdR5wcxzHyLz
@vercel

vercel Bot commented Apr 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
moneyplanner Ready Ready Preview, Comment Apr 22, 2026 0:19am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants